Make passing list type arguments to k4run easier and add tests to verify parsing #164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tmadlener
force-pushed
the
k4run-better-list-args
branch
from
19887a7 to
d187a38
Compare
Merged
2 tasks
tmadlener
force-pushed
the
k4run-better-list-args
branch
from
d187a38 to
51cee6c
Compare
|
As just discussed during the Key4hep meeting, we might want to go in the complete opposite direction with k4run, i.e. simplifying the argument parsing and removing the possibility to touch all the parameters of all the algorithms, in favor of using from k4FWCore.parseArgs import parser
parser.add_arguments("--algoParam", help="My special parameter")
my_args = parser.parse_known_args()[0]
# ...
myAlgo.specialParameter = my_args.algoParamI will mark this as draft in the meantime and open an issue to not forget about documenting the possibility of using |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BEGINRELEASENOTES
k4runforGaudi::Property<vector<T>>algorithm properties. This is a breaking change, previously the same was possible with adding to the list by passing the argument multiple times with different valuesENDRELEASENOTES
Although this is a breaking change, I think this is an improvement to the current situation because
vector<string>property that is defaulted to be empty. What makes this special is that e.g. thePodioInput.collectionsproperties is such a list of strings and currently it would be impossible to change that from the command line.I am not sure how many actual workflows this currently breaks. I think / hope not too many. In the end my major motivation at the moment is actually for testing, where this would be extremely handy to avoid having to write a second options file just to change effectively one list of values.